Extraction of common scripts and utilities + cleanup and imrpovements #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
drobison00
added
the
enhancement
drobison00
added
non-breaking
drobison00
changed the title
mdemoret-nv
requested changes
Dec 14, 2022
There was a problem hiding this comment.
Some general comments:
- I think there is a lot in the
cifolder which could be moved to utilities. Many of theci/scriptsfiles are nearly the same. We could include that in this PR or separate into a second PR - The files in the
condaanddockerdirectories seem to be off. Many of the docker related files should be moved to the docker directory - We can get rid of the files in
misc - We should use this as a good time to enforce consistent file names. i.e. all lower camel case.
- Since this will be used by multiple repos, we should ensure that there are no unintended side effects. For example, if you do
include(cmake/morpheus_utils/load)it checks for pybind11, python and skbuild. These arent necessary for all repos but will be required now.- We should try to enforce that loading this repo doesnt have any effects until specific functions are called. i.e. the
register_apifiles just create macros/functions but dont run anything.
- We should try to enforce that loading this repo doesnt have any effects until specific functions are called. i.e. the
cmake/morpheus_utils/package_config/boost/Configure_boost.cmake
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/package_config/cudf/patches/expose_cudf_column_info.patch
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/environment_config/rapids_cmake/register_api.cmake
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/environment_config/rapids_cmake/register_api.cmake
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/environment_config/rapids_cmake/register_api.cmake
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/environment_config/rapids_cmake/register_api.cmake
Outdated
Show resolved
Hide resolved
cmake/morpheus_utils/environment_config/rapids_cmake/register_api.cmake
Outdated
Show resolved
Hide resolved
mdemoret-nv
reviewed
Dec 14, 2022
cmake/morpheus_utils/environment_config/iwyu/register_api.cmake
Outdated
Show resolved
Hide resolved
…ackage search path
mdemoret-nv
approved these changes
Jan 6, 2023
|
/merge |
ghost
merged commit 
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Consolidates common functionality for cmake, scripts, conda, debugging, etc.. from nv-morpheus/MRC and nv-morpheus/Morpheus.
CMake tools can be added at various levels of completeness, and should satisfy their own pre-requisites at each level.
include(cmake/morpheus_utils/load): Enables all morpheus_utils_xxx library callsinclude(cmake/morpheus_utils/.../register_api): Enables morpheus_utils_xx library calls for the given sub-librarycmake/morpheus_utils/
environment_config- contains items related to build environment, compiler setup, cmake configuration, caching, rapids_cpm, etc...general- General helpers, tools, debugging code, etc..grpc- GRPC helpers, scripts, and definitionspackage_config- Wrapper functions for pulling, loading, and building various libraries that nv-morpheus repositories use.package_search- FindXXX filespython- Python specific helper tools, pybind11 utilities, macros, functions, and utilitiescmake/conda
Nothing currently; while both repos duplicate code, there are specific and subtle differences between each. Consolidating is better left to its own PR.
cmake/docker
Similar to conda, much of the duplicated code doesn't have a clear path to generalizing/merging. Consolidation here is also better left to be its own PR.
cmake/misc
Items that were either unused in either repo but we want to retain, or artifacts that had no clear home.
cmake/scripts
Common scripts for bash, python, etc.. that can be shared across repositories.